Skip to content

planner, executor, ddl: normalize materialized view log privilege targets and checks#68819

Merged
ti-chi-bot[bot] merged 3 commits into
pingcap:feature/release-8.5-materialized-view-2603from
solotzg:mv-priv
Jun 2, 2026
Merged

planner, executor, ddl: normalize materialized view log privilege targets and checks#68819
ti-chi-bot[bot] merged 3 commits into
pingcap:feature/release-8.5-materialized-view-2603from
solotzg:mv-priv

Conversation

@solotzg
Copy link
Copy Markdown
Contributor

@solotzg solotzg commented May 31, 2026

What problem does this PR solve?

ref #68518

Problem Summary:

Materialized view log privilege checks were not fully consistent across planner/executor/DDL paths:

  • Some paths used manual $mlog$... name construction, which risks divergence.
  • Privilege target objects and error messages were not fully aligned for MLog-related statements.
  • Extension points for future ALTER MATERIALIZED VIEW LOG column-level actions were missing in key paths.

What changed and how does it work?

  1. Normalize MLog physical-table name resolution
  • Replace scattered manual MLog-name construction with the dedicated helper:
    model.MaterializedViewLogTableName(...).
  • Apply this in planner, executor, DDL, and schema tracker related paths.
  1. Align privilege checks to resolved MLog target objects
  • For MLog-related DDL/SHOW/PURGE/CANCEL flows, checks consistently resolve to the physical MLog object.
  • Keep PURGE MATERIALIZED VIEW LOG executor-side OPERATE VIEW verification on resolved MLog.
  • Keep cancel-job precheck resolving running job to concrete MV/MLog object before privilege verification.
  1. Unify user-facing behavior and expected errors
  • Adjust related error messages and privilege-denied targets to be consistent with resolved MLog object semantics.
  • Update integration and unit test expectations accordingly.
  1. Add TODOs on key extension paths
  • Add focused TODOs in parser/planner/DDL key points for future support of column-level
    ALTER MATERIALIZED VIEW LOG actions and action-specific privilege checks.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed privilege enforcement for materialized view log operations. Privileges like CREATE VIEW, DROP, ALTER, and SHOW VIEW are now correctly checked against the materialized view log object itself, rather than against base tables or dependent materialized views, providing more consistent and predictable authorization behavior.

solotzg added 3 commits May 31, 2026 20:50
…sages

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>
… dedicated function

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>
…IZED VIEW LOG actions and action-specific privilege checks

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>
@ti-chi-bot ti-chi-bot Bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. sig/planner SIG: Planner labels May 31, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors how materialized view log (MLOG) tables are named and authorized across the system. A new centralized helper replaces scattered "$mlog$" string concatenations, and privilege enforcement is redirected from base tables to the internal $mlog$ pseudo-table itself for all DDL, SHOW, GRANT, and purge operations.

Changes

Materialized View Log Privilege and Naming Consolidation

Layer / File(s) Summary
MLOG Table Name Centralization
pkg/meta/model/table.go, pkg/ddl/executor.go, pkg/ddl/materialized_view.go, pkg/ddl/schematracker/checker.go, pkg/ddl/schematracker/dm_tracker.go, pkg/executor/materialized_view.go
Introduces MaterializedViewLogTableNamePrefix constant and MaterializedViewLogTableName(baseTableName) helper function; replaces all manual "$mlog$" + ... concatenations with centralized calls throughout DDL and executor code paths.
Planner Privilege Policy for MLOG Operations
pkg/planner/core/planbuilder.go
Updates buildShow and buildDDL to record privilege checks against MLOG pseudo-table names: SHOW CREATE MATERIALIZED VIEW LOG and SHOW ... WAIT PURGE check SHOW VIEW/SELECT on the MLOG; CREATE/DROP/ALTER MLOG use CREATE VIEW/DROP/ALTER privileges on the derived MLOG name instead of base-table permissions. Introduces materializedViewLogNameForBaseTable helper.
Executor Privilege Enforcement and Visibility
pkg/executor/grant.go, pkg/executor/show.go
isSafeMLogTableGrantPriv now permits the same privilege set as materialized views (materializedViewTablePrivs); tablePrivsForGrantTarget returns materializedViewTablePrivs for MLOG grants. fetchShowMaterializedViewLogs removes base-table database lookups and checks visibility using hasAnyMaterializedViewVisiblePriv against the MLOG name instead of base-table SELECT.
Purge and Cancel Job Authorization Redirect
pkg/executor/materialized_view.go
checkCancelMaterializedViewJobPrivilege and resolveCancelPurgeJobPrivilegeTarget now resolve and validate OPERATE VIEW privilege directly against the MLOG metadata; executePurgeMaterializedViewLog switches from validating against dependent materialized views to validating against the MLOG itself.
Unit Test Updates for MLOG Privileges
pkg/executor/test/ddl/materialized_view_ddl_test.go, pkg/executor/test/ddl/mview_log_ddl_test.go
Adds authorization scenarios for SHOW MATERIALIZED VIEW LOGS based on MLOG table privileges; updates CREATE VIEW, DROP, ALTER, and SHOW CREATE MLOG privilege tests to validate against $mlog$ pseudo-table; introduces new TestDropMaterializedViewLogPrivilege test; adjusts purge/cancel privilege grants to target MLOG objects.
Integration Test Updates for MLOG Privilege Scenarios
tests/integrationtest/t/executor/mview_privilege.test, tests/integrationtest/r/executor/mview_privilege.result
Comprehensive test script and result updates: SHOW VIEW, SELECT, CREATE VIEW, ALTER, DROP, and OPERATE VIEW grants/revokes now target $mlog$ pseudo-tables; expected error messages and privilege denial contexts updated to reference MLOG objects instead of base tables or dependent views; error code for purge_global scenario updated from 1105 to 1142.
Future Refactoring TODO
pkg/parser/ast/ddl.go
Adds a TODO comment documenting the intent to support column-level ALTER MATERIALIZED VIEW LOG actions in future development.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • pingcap/tidb#68518: Overlaps on materialized-view-log privilege matrix work in grant.go and materialized_view.go, particularly privilege set definitions and MLOG authorization scoping.
  • pingcap/tidb#68691: Adds SHOW MATERIALIZED VIEW ... REMAIN_LOGS and SHOW MATERIALIZED VIEW LOG ... WAIT_PURGE statements whose privilege targeting is updated by this PR.
  • pingcap/tidb#68698: Implements SHOW CREATE MATERIALIZED VIEW LOG support whose privilege resolution for the $mlog$ object is directly aligned with this PR's changes to planbuilder.go and show.go.

Suggested labels

sig/planner, size/XXL, release-note-none

Suggested reviewers

  • windtalker
  • wshwsh12
  • winoros
  • gengliqi

Poem

🐰 From "$mlog$" + name scattered wide,
A helper function brings them unified.
Privileges shift from base to MLOG's own land—
Authorization now in the logger's hand.
Each GRANT and DROP sings true, centralized and grand! 🎭

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: normalizing materialized view log privilege targets and checks across multiple packages.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is comprehensive and clearly addresses the problem, solution approach, and includes all required sections per the template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tiprow
Copy link
Copy Markdown

tiprow Bot commented May 31, 2026

Hi @solotzg. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 31, 2026

Codecov Report

❌ Patch coverage is 0% with 70 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feature/release-8.5-materialized-view-2603@7a81553). Learn more about missing BASE report.

Additional details and impacted files
@@                               Coverage Diff                               @@
##             feature/release-8.5-materialized-view-2603     #68819   +/-   ##
===============================================================================
  Coverage                                              ?   22.4152%           
===============================================================================
  Files                                                 ?       1712           
  Lines                                                 ?     645154           
  Branches                                              ?          0           
===============================================================================
  Hits                                                  ?     144613           
  Misses                                                ?     482441           
  Partials                                              ?      18100           
Flag Coverage Δ
integration 22.4152% <0.0000%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling ∅ <0.0000%> (?)
parser ∅ <0.0000%> (?)
br 23.5563% <0.0000%> (?)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@windtalker windtalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ti-chi-bot ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 1, 2026
@solotzg
Copy link
Copy Markdown
Contributor Author

solotzg commented Jun 2, 2026

/retest

@tiprow
Copy link
Copy Markdown

tiprow Bot commented Jun 2, 2026

@solotzg: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ti-chi-bot ti-chi-bot Bot added the lgtm label Jun 2, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jun 2, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gengliqi, windtalker
Once this PR has been reviewed and has the lgtm label, please assign henrybw, tangenta for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jun 2, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jun 2, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-06-01 01:33:49.545831284 +0000 UTC m=+146130.616148684: ☑️ agreed by windtalker.
  • 2026-06-02 06:21:47.997029486 +0000 UTC m=+249809.067346896: ☑️ agreed by gengliqi.

@windtalker
Copy link
Copy Markdown
Contributor

/override idc-jenkins-ci-tidb/mysql-test pull-br-integration-test pull-unit-test-ddlv1 idc-jenkins-ci-tidb/check_dev idc-jenkins-ci-tidb/check_dev_2 idc-jenkins-ci-tidb/unit-test

@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jun 2, 2026

@windtalker: Overrode contexts on behalf of windtalker: idc-jenkins-ci-tidb/check_dev, idc-jenkins-ci-tidb/check_dev_2, idc-jenkins-ci-tidb/mysql-test, idc-jenkins-ci-tidb/unit-test, pull-br-integration-test, pull-unit-test-ddlv1

Details

In response to this:

/override idc-jenkins-ci-tidb/mysql-test pull-br-integration-test pull-unit-test-ddlv1 idc-jenkins-ci-tidb/check_dev idc-jenkins-ci-tidb/check_dev_2 idc-jenkins-ci-tidb/unit-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ti-chi-bot ti-chi-bot Bot merged commit e8c6e40 into pingcap:feature/release-8.5-materialized-view-2603 Jun 2, 2026
21 of 22 checks passed
@solotzg solotzg deleted the mv-priv branch June 2, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants